-
Notifications
You must be signed in to change notification settings - Fork 8k
shell_uart: Stop polling if buffer is full #97236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
shell_uart: Stop polling if buffer is full #97236
Conversation
Hello @sjlongland, and thank you very much for your first pull request to the Zephyr project! |
6e88425
to
c5843ce
Compare
subsys/shell/backends/shell_uart.c
Outdated
while (uart_poll_in(sh_uart->common.dev, &c) == 0) { | ||
while ((ring_buf_space_get(&sh_uart->rx_ringbuf) > 0) && | ||
(uart_poll_in(sh_uart->common.dev, &c) == 0)) { | ||
if (ring_buf_put(&sh_uart->rx_ringbuf, &c, 1) == 0U) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the if
statement still make sense to have it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it there just in case there was some possibility of concurrency in the shell thread… I'm still getting my head around the architecture that is Zephyr, but I can remove it if you think that'd be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in c14fdd5. :-)
"CORE-5615 / GHI-97235" - what does this refer to? |
One's an internal Jira tracking number (which I can remove). GHI stands for GitHub Issue. i.e. Git Hub Issue #97235. |
at leasr one of them is obvious #97235 :) |
Please remove it from the commit subject, if needed put a full link in the commit body. |
Before calling `uart_poll_in`, check we have space to store the character in the ring buffer beforehand. If we do, *then* poll for the character. That way we don't miss out on serial traffic when our ring buffer is full unless we fill our hardware ring buffer too. Signed-off-by: Stuart Longland <[email protected]>
c5843ce
to
fdb36c9
Compare
We're now checking if there is space prior to executing the loop body, therefore guaranteeing this condition is never going to fire in a single-threaded context. Signed-off-by: Stuart Longland <[email protected]>
Yep, I've removed it from the commit message. |
|
Before calling
uart_poll_in
, check we have space to store the character in the ring buffer beforehand. If we do, then poll for the character.That way we don't miss out on serial traffic when our ring buffer is full unless we fill our hardware ring buffer too.
Fixes issue #97235.